Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Add Slices" modal on dashboard page #678

Merged
merged 4 commits into from
Jul 8, 2016
Merged

Conversation

georgeke
Copy link
Contributor

screen shot 2016-06-26 at 5 44 08 pm

Notes:

  • only shows slices owned by the logged in user
  • will save the dashboard & refresh the page after adding slices

@coveralls
Copy link

coveralls commented Jun 27, 2016

Coverage Status

Coverage decreased (-0.2%) to 81.084% when pulling bc2330ea67b08f17deb6d20eb65570ba24ab0adb on georgeke:add-slice into 4191b75 on airbnb:master.

@coveralls
Copy link

coveralls commented Jun 27, 2016

Coverage Status

Coverage increased (+0.1%) to 81.431% when pulling 1e09d28a150aee82c51eeec814f107650eb47bb7 on georgeke:add-slice into 4191b75 on airbnb:master.

@mistercrunch
Copy link
Member

LGTM on the python side, @ascott, do you have input on the React side?

@@ -182,15 +167,148 @@ class GridLayout extends React.Component {
margin={[20, 20]}
useCSSTransforms={false}
draggableHandle=".drag">
{this.state.sliceElements}
{this.state.slices.map(function (slice) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use es6 fat arrow here and you then you won't need to bind this.

this.state.slices.map((slice) =>  {
  // do things with slice
});

@ascott
Copy link

ascott commented Jun 28, 2016

are the dark grey rows the selected slices? i wonder if we should use checkboxes here since that seems like a pattern we are using in most other tables where you can select a row.

i think we could get rid of the show x entries drop down. since this is a modal, we don't want it to get to long if someone loads 20/50 entries. we could figure out what a good max height should be for the modal and then only show that number of slices and paginate the rest.

<button type="button" className="close" data-dismiss="modal" aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
<h4 className="modal-title" id="myModalLabel">Add New Slices</h4>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this id used for anything?

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage increased (+0.1%) to 81.141% when pulling 5a3a84e5d23afc785ee2b8744cb2d92a22a46660 on georgeke:add-slice into 8135c24 on airbnb:master.

@georgeke
Copy link
Contributor Author

georgeke commented Jul 7, 2016

@ascott

@coveralls
Copy link

coveralls commented Jul 8, 2016

Coverage Status

Coverage increased (+0.1%) to 81.141% when pulling ac63d93 on georgeke:add-slice into afff788 on airbnb:master.

@georgeke georgeke merged commit 04f3e3b into apache:master Jul 8, 2016
@georgeke georgeke deleted the add-slice branch July 8, 2016 04:40
customButtons: PropTypes.node
};

class Modal extends React.Component {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be great to move this component to caravel/assets/javascripts/components/Modal.jsx so we can reuse it across the app.

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021
Bumps [whatwg-fetch](https://github.com/github/fetch) from 3.0.0 to 3.2.0.
- [Release notes](https://github.com/github/fetch/releases)
- [Commits](JakeChampion/fetch@v3.0.0...v3.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021
Bumps [whatwg-fetch](https://github.com/github/fetch) from 3.0.0 to 3.2.0.
- [Release notes](https://github.com/github/fetch/releases)
- [Commits](JakeChampion/fetch@v3.0.0...v3.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021
Bumps [whatwg-fetch](https://github.com/github/fetch) from 3.0.0 to 3.2.0.
- [Release notes](https://github.com/github/fetch/releases)
- [Commits](JakeChampion/fetch@v3.0.0...v3.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021
Bumps [whatwg-fetch](https://github.com/github/fetch) from 3.0.0 to 3.2.0.
- [Release notes](https://github.com/github/fetch/releases)
- [Commits](JakeChampion/fetch@v3.0.0...v3.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.11.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants